Fix issue when FluenceJS was not working in webpack-based web projects #176
Conversation
ci.js
Outdated
| async function getFiles(currentPath, files) { | ||
| const entries = await fs.readdir(currentPath, { withFileTypes: true }); | ||
|
|
||
| for (let file of entries) { | ||
| if (file.name === "node_modules") { | ||
| continue; | ||
| } | ||
|
|
||
| if (file.isDirectory()) { | ||
| await getFiles(`${currentPath}${file.name}/`, files); | ||
| } else if (file.name === "package.json") { | ||
| const packageJsonPath = path.join( | ||
| __dirname, | ||
| currentPath, | ||
| file.name | ||
| ); | ||
| files.push(packageJsonPath); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
With help of something like this, we can avoid await inside the loop. Also, I suggest renaming this function, so it's more clear what it does, remove the need to mutate function argument and avoid using template-literal path construction (use join instead). I would also suggest avoiding using __dirname and use process.cwd() or something else because __dirname not supported by ECMAScript modules, so it's not future-proof
const getPackageJsonsRecursive = async (currentPath) => (await Promise.all(
(await fs.readdir(currentPath, { withFileTypes: true }))
.filter(({name}) => name !== "node_modules" && (file.isDirectory() || file.name === "package.json"))
.map(() => file.isDirectory()
? getPackageJsonsRecursive(path.join(currentPath, file.name))
: Promise.resolve([
path.join(
__dirname,
currentPath,
file.name
)
])
)
)).flat()
| @@ -0,0 +1,45 @@ | |||
| { | |||
There was a problem hiding this comment.
I suggest not to commit this folder with react test since it's not working
| import { isBrowser } from 'browser-or-node'; | ||
| import { Buffer as BufferPolyfill } from 'buffer'; | ||
|
|
||
| export default isBrowser ? BufferPolyfill : Buffer; |
There was a problem hiding this comment.
Are you completely sure all of this in needed? As I remember, this polyfill should just work if you import it without any additional stuff. Please make sure we are not doing extra work here
There was a problem hiding this comment.
This code was meant to include polyfill only for browser usage of the library. However it seems like it results in always using the polyfill.
I decided to just leave a single import from 'buffer' to make things clearer
shamsartem
left a comment
shamsartem
left a comment
There was a problem hiding this comment.
I left some comments in ci.js file, I think changes I propose are not very hard but can measurably improve the speed of that job. Also left a question in Buffer.ts
I also suggest removing rect_test since it's not working, if for some reason somebody checks out his commit he will find not working code which is not cool I think
React test doesn't work, so it has to be skipped (
package.json.skip). It is somehow related to pnpm specific and works well in https://github.com/fluencelabs/examples. Will be fixed in separate PR.